-
Notifications
You must be signed in to change notification settings - Fork 2k
S3 Vector Store implementation #4031
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
S3 Vector Store implementation #4031
Conversation
|
Hey @ilayaperumalg , before I polish everything fully and write docs. For All of the code is done just making it more readable and fixing some imports here and there. |
11e1dbc to
4834be4
Compare
Signed-off-by: matejnedic <[email protected]>
4834be4 to
5a1cf4a
Compare
Signed-off-by: matejnedic <[email protected]>
Signed-off-by: matejnedic <[email protected]>
|
Hey @ilayaperumalg , is there any way I can help with getting this PR approved? I can provide mannual tests against real AWS and provide Spring Team with test findings or even take ownership on testing and reviewing future S3 Vector Store additions in this repo? |
|
@MatejNedic Sorry about the delay in response. We'll try to get this reviewed soon. |
|
@MatejNedic Thanks for this implementation. We are starting the review on this PR. I have couple of questions. Are there any updates to the testing issues you mentioned? We really need to add some integration tests to verify the basic vector store behavior before we can merge the PR. Does S3 not yet support a testing environment? On that note, if it is only available in preview mode still, we might rather wait before we push this as a production feature in Spring AI. Anyways, could you let us know your latest findings on this topic? Can you rebase your PR with the latest main branch? Also, can you fix all the checkstyle issues during the build? Thanks! |
|
Hi @sobychacko, Thanks for starting the review and for the feedback. Unfortunately, Localstack does not yet support S3 Vector Store, which makes end to end integration testing impossible without using a real AWS environment. I considered spinning up Localstack in TestContainers once support becomes available, but for now, that isn’t an option. To ensure the behavior is verified, I can write full system tests against real AWS infrastructure and either demo the results during a meeting or record and share a short video walkthrough. That said, if you're okay with it, I'd prefer to park this feature for a bit. I'm currently fully focused on finalizing Spring Cloud AWS 4.0.0-RCx, aiming to align the release with the upcoming Spring Cloud Release Train. Once I get through the bulk of that work, I’ll return to this PR, rebase it, fix the checkstyle issues, and include the system test showcase. Let me know if this approach works for you or if you'd prefer I prioritize things differently. Thanks again! |
|
@MatejNedic That sounds absolutely fine. We can park it here and please get back to it whenever you are ready. We can target this as a feature item for the Thanks! |
Before writing docs sadly there is no yet support for S3 Vector Store meaning I can do mock tests only. Is that fine or to skip mock tests completly?
#3834